Skip to content

Add fetch for tenant config and write context file#314

Open
timothyF95 wants to merge 11 commits intomainfrom
add-context-file-fetch
Open

Add fetch for tenant config and write context file#314
timothyF95 wants to merge 11 commits intomainfrom
add-context-file-fetch

Conversation

@timothyF95
Copy link
Contributor

@timothyF95 timothyF95 commented Mar 17, 2026

@timothyF95 timothyF95 marked this pull request as ready for review March 17, 2026 08:00
@timothyF95 timothyF95 requested a review from a team as a code owner March 17, 2026 08:00
case "OWNER_KEY_SIGNING":
flows = append(flows, "owner-key-signing")
default:
flows = append(flows, strings.ToLower(f))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we raise an error in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to raise an error at this point because it could cause a failure while we are still developing the feature. I have added a debug log

}

// EnsureContext guarantees the registry manifest exists for the current environment.
// API key users always fetch fresh; bearer token users use the cached file from login.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the JWT expires, do we need to refetch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the JWT has expired the refresh token will automatically request another one. If the refresh token has expired the auth package will prompt the user to log in again which will refresh the context

}{
{"0xaE55eB3EDAc48a1163EE2cbb1205bE1e90Ea1135", "0xaE55...1135"},
{"0x12345678", "0x12345678"}, // 10 chars, no abbreviation
{"short", "short"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this test case make sense? short is not a valid ETH address

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are not yet certain on the label and it may change in the future so I think it would be good to cater for man cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants